Limit the meaning of "custom element" to not include `is` #26524

sebmarkbage · 2023-03-30T20:43:03Z

This PR has a bunch of surrounding refactoring. See individual commits.

The main change is that we no longer special case typeof is === 'string' as a special case according to the enableCustomElementPropertySupport flag.

Effectively this means that you can't use custom properties/events, other than the ones React knows about on <input is="my-input"> extensions.

This is unfortunate but there's too many paths that are forked in inconsistent ways since we fork based on tag name. I think the solution is to let all React elements set unknown properties/events in the same way as this flag but that's a bigger change than this flag implies.

Since is is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work.

We still support passing the is prop and turn that into the appropriate createElement call.

@josepharhar

react-sizebot · 2023-03-30T20:46:42Z

Comparing: 1a1d61f...d4e2063

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	162.96 kB	162.81 kB	=	51.35 kB	51.35 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	164.61 kB	164.35 kB	=	51.84 kB	51.84 kB
facebook-www/ReactDOM-prod.classic.js	=	557.72 kB	556.72 kB	=	98.34 kB	98.21 kB
facebook-www/ReactDOM-prod.modern.js	=	541.42 kB	540.41 kB	=	95.85 kB	95.83 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against d4e2063

Makes it clearer what the subtle difference is. Only do this when the flag is on. Otherwise, use setValueForAttribute directly

We don't need the hoistable type to use `is` since that's not supported.

Since we know the namespace in this context, we don't need the special list of SVG/MathML tags that have dashes in them. Any `is` should also have a known type and use lower case for the type tag so it's irrelevant for this check.

This can only be specified once and is really technically part of the key just like the tag name. We just don't have a way to model that in React so we warn.

That's the technical term.

Effectively this means that you can't use custom properties/events, other than the ones React knows about on `is` extensions. This is unfortunate but there's too many paths that are forked in inconsistent ways. I think the solution is to let all React elements set unknown properties in the same way as this flag but that's a bigger change than this flag implies. Since `is` is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work.

@josepharhar

This PR has a bunch of surrounding refactoring. See individual commits. The main change is that we no longer special case `typeof is === 'string'` as a special case according to the `enableCustomElementPropertySupport` flag. Effectively this means that you can't use custom properties/events, other than the ones React knows about on `<input is="my-input">` extensions. This is unfortunate but there's too many paths that are forked in inconsistent ways since we fork based on tag name. I think __the solution is to let all React elements set unknown properties/events in the same way as this flag__ but that's a bigger change than this flag implies. Since `is` is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work. We still support passing the `is` prop and turn that into the appropriate createElement call. @josepharhar DiffTrain build for [43a70a6](43a70a6)

Summary: This sync includes the following changes: - **[ca01f359b](facebook/react@ca01f359b )**: Remove skipUnmountedBoundaries ([#26489](facebook/react#26489)) //<Ricky>// - **[43a70a610](facebook/react@43a70a610 )**: Limit the meaning of "custom element" to not include `is` ([#26524](facebook/react#26524)) //<Sebastian Markbåge>// - **[1308e49a6](facebook/react@1308e49a6 )**: [Flight Plugin] Scan for "use client" ([#26474](facebook/react#26474)) //<dan>// - **[1a1d61fed](facebook/react@1a1d61fed )**: Warn for ARIA typos on custom elements ([#26523](facebook/react#26523)) //<Sebastian Markbåge>// - **[73deff0d5](facebook/react@73deff0d5 )**: Refactor DOMProperty and CSSProperty ([#26513](facebook/react#26513)) //<Sebastian Markbåge>// - **[2d51251e6](facebook/react@2d51251e6 )**: Clean up deferRenderPhaseUpdateToNextBatch ([#26511](facebook/react#26511)) //<Andrew Clark>// - **[0ffc7f632](facebook/react@0ffc7f632 )**: Update useMemoCache test to confirm that cache persists across errors ([#26510](facebook/react#26510)) //<Joseph Savona>// - **[29a3be78b](facebook/react@29a3be78b )**: Move ReactDOMFloat to react-dom/src/ ([#26514](facebook/react#26514)) //<Sebastian Markbåge>// - **[4c2fc0190](facebook/react@4c2fc0190 )**: Generate safe javascript url instead of throwing with disableJavaScriptURLs is on ([#26507](facebook/react#26507)) //<Sebastian Markbåge>// - **[f0aafa1a7](facebook/react@f0aafa1a7 )**: Convert a few more tests to waitFor test helpers ([#26509](facebook/react#26509)) //<Andrew Clark>// - **[90995ef8b](facebook/react@90995ef8b )**: Delete "triangle" resuming fuzz tester ([#26508](facebook/react#26508)) //<Andrew Clark>// - **[f118b7ceb](facebook/react@f118b7ceb )**: [Flight] Gated test for dropped transport of undefined object values ([#26478](facebook/react#26478)) //<Sebastian Silbermann>// - **[fd0511c72](facebook/react@fd0511c72 )**: [Flight] Add support BigInt support ([#26479](facebook/react#26479)) //<Sebastian Silbermann>// - **[85de6fde5](facebook/react@85de6fde5 )**: Refactor DOM special cases per tags including controlled fields ([#26501](facebook/react#26501)) //<Sebastian Markbåge>// - **[1f5cdf8c7](facebook/react@1f5cdf8c7 )**: Update Suspense fuzz tests to use `act` ([#26498](facebook/react#26498)) //<Andrew Clark>// - **[f62cb39ee](facebook/react@f62cb39ee )**: Make disableSchedulerTimeoutInWorkLoop a static ff ([#26497](facebook/react#26497)) //<Ricky>// - **[41b4714f1](facebook/react@41b4714f1 )**: Remove disableNativeComponentFrames ([#26490](facebook/react#26490)) //<Ricky>// - **[fc90eb636](facebook/react@fc90eb636 )**: Codemod more tests to waitFor pattern ([#26494](facebook/react#26494)) //<Andrew Clark>// - **[e0bbc2662](facebook/react@e0bbc2662 )**: Improve tests that deal with microtasks ([#26493](facebook/react#26493)) //<Andrew Clark>// - **[8faf75193](facebook/react@8faf75193 )**: Codemod some expiration tests to waitForExpired ([#26491](facebook/react#26491)) //<Andrew Clark>// - **[8342a0992](facebook/react@8342a0992 )**: Remove unused feature flag disableSchedulerTimeoutBasedOnReactExpirationTime ([#26488](facebook/react#26488)) //<Jan Kassens>// - **[afea1d0c5](facebook/react@afea1d0c5 )**: [flow] make Flow suppressions explicit on the error ([#26487](facebook/react#26487)) //<Jan Kassens>// - **[768f965de](facebook/react@768f965de )**: Suspensily committing a prerendered tree ([#26434](facebook/react#26434)) //<Andrew Clark>// - **[d12bdcda6](facebook/react@d12bdcda6 )**: Fix Flow types of useEffectEvent ([#26468](facebook/react#26468)) //<Sebastian Silbermann>// - **[73b6435ca](facebook/react@73b6435ca )**: [Float][Fiber] Implement waitForCommitToBeReady for stylesheet resources ([#26450](facebook/react#26450)) //<Josh Story>// - **[175962c10](facebook/react@175962c10 )**: Fix remaining CommonJS imports after Rollup upgrade ([#26473](facebook/react#26473)) //<dan>// - **[909c6dacf](facebook/react@909c6dacf )**: Update Rollup to 3.x ([#26442](facebook/react#26442)) //<Mark Erikson>// - **[9c54b29b4](facebook/react@9c54b29b4 )**: Remove ReactFabricPublicInstance and used definition from ReactNativePrivateInterface ([#26437](facebook/react#26437)) //<Rubén Norte>// - **[f77099b6f](facebook/react@f77099b6f )**: Remove layout effect warning on the server ([#26395](facebook/react#26395)) //<Ricky>// - **[51a7c45f8](facebook/react@51a7c45f8 )**: Bugfix: SuspenseList incorrectly forces a fallback ([#26453](facebook/react#26453)) //<Andrew Clark>// - **[afb3d51dc](facebook/react@afb3d51dc )**: Fix enableClientRenderFallbackOnTextMismatch flag ([#26457](facebook/react#26457)) //<Sebastian Markbåge>// - **[8e17bfd14](facebook/react@8e17bfd14 )**: Make InternalInstanceHandle type opaque in ReactNativeTypes ([#26461](facebook/react#26461)) //<Rubén Norte>// - **[b93b4f074](facebook/react@b93b4f074 )**: Should not throw for children of iframe or object ([#26458](facebook/react#26458)) //<Sebastian Markbåge>// - **[c0b34bc5f](facebook/react@c0b34bc5f )**: chore: update links of docs and api ([#26455](facebook/react#26455)) //<Leedom>// - **[ffb6733ee](facebook/react@ffb6733ee )**: fix docs link for useSyncExternalStore ([#26452](facebook/react#26452)) //<Valor(华洛)>// - **[12a1d140e](facebook/react@12a1d140e )**: Don't prerender siblings of suspended component ([#26380](facebook/react#26380)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 77ba161...ca01f35 jest_e2e[run_all_tests] bypass-github-export-checks Reviewed By: sammy-SC Differential Revision: D44669450 fbshipit-source-id: f160aad4719a00df3ceeca78d5f3fcd0aa0f8437

Summary: This sync includes the following changes: - **[ca01f359b](facebook/react@ca01f359b )**: Remove skipUnmountedBoundaries ([facebook#26489](facebook/react#26489)) //<Ricky>// - **[43a70a610](facebook/react@43a70a610 )**: Limit the meaning of "custom element" to not include `is` ([facebook#26524](facebook/react#26524)) //<Sebastian Markbåge>// - **[1308e49a6](facebook/react@1308e49a6 )**: [Flight Plugin] Scan for "use client" ([facebook#26474](facebook/react#26474)) //<dan>// - **[1a1d61fed](facebook/react@1a1d61fed )**: Warn for ARIA typos on custom elements ([facebook#26523](facebook/react#26523)) //<Sebastian Markbåge>// - **[73deff0d5](facebook/react@73deff0d5 )**: Refactor DOMProperty and CSSProperty ([facebook#26513](facebook/react#26513)) //<Sebastian Markbåge>// - **[2d51251e6](facebook/react@2d51251e6 )**: Clean up deferRenderPhaseUpdateToNextBatch ([facebook#26511](facebook/react#26511)) //<Andrew Clark>// - **[0ffc7f632](facebook/react@0ffc7f632 )**: Update useMemoCache test to confirm that cache persists across errors ([facebook#26510](facebook/react#26510)) //<Joseph Savona>// - **[29a3be78b](facebook/react@29a3be78b )**: Move ReactDOMFloat to react-dom/src/ ([facebook#26514](facebook/react#26514)) //<Sebastian Markbåge>// - **[4c2fc0190](facebook/react@4c2fc0190 )**: Generate safe javascript url instead of throwing with disableJavaScriptURLs is on ([facebook#26507](facebook/react#26507)) //<Sebastian Markbåge>// - **[f0aafa1a7](facebook/react@f0aafa1a7 )**: Convert a few more tests to waitFor test helpers ([facebook#26509](facebook/react#26509)) //<Andrew Clark>// - **[90995ef8b](facebook/react@90995ef8b )**: Delete "triangle" resuming fuzz tester ([facebook#26508](facebook/react#26508)) //<Andrew Clark>// - **[f118b7ceb](facebook/react@f118b7ceb )**: [Flight] Gated test for dropped transport of undefined object values ([facebook#26478](facebook/react#26478)) //<Sebastian Silbermann>// - **[fd0511c72](facebook/react@fd0511c72 )**: [Flight] Add support BigInt support ([facebook#26479](facebook/react#26479)) //<Sebastian Silbermann>// - **[85de6fde5](facebook/react@85de6fde5 )**: Refactor DOM special cases per tags including controlled fields ([facebook#26501](facebook/react#26501)) //<Sebastian Markbåge>// - **[1f5cdf8c7](facebook/react@1f5cdf8c7 )**: Update Suspense fuzz tests to use `act` ([facebook#26498](facebook/react#26498)) //<Andrew Clark>// - **[f62cb39ee](facebook/react@f62cb39ee )**: Make disableSchedulerTimeoutInWorkLoop a static ff ([facebook#26497](facebook/react#26497)) //<Ricky>// - **[41b4714f1](facebook/react@41b4714f1 )**: Remove disableNativeComponentFrames ([facebook#26490](facebook/react#26490)) //<Ricky>// - **[fc90eb636](facebook/react@fc90eb636 )**: Codemod more tests to waitFor pattern ([facebook#26494](facebook/react#26494)) //<Andrew Clark>// - **[e0bbc2662](facebook/react@e0bbc2662 )**: Improve tests that deal with microtasks ([facebook#26493](facebook/react#26493)) //<Andrew Clark>// - **[8faf75193](facebook/react@8faf75193 )**: Codemod some expiration tests to waitForExpired ([facebook#26491](facebook/react#26491)) //<Andrew Clark>// - **[8342a0992](facebook/react@8342a0992 )**: Remove unused feature flag disableSchedulerTimeoutBasedOnReactExpirationTime ([facebook#26488](facebook/react#26488)) //<Jan Kassens>// - **[afea1d0c5](facebook/react@afea1d0c5 )**: [flow] make Flow suppressions explicit on the error ([facebook#26487](facebook/react#26487)) //<Jan Kassens>// - **[768f965de](facebook/react@768f965de )**: Suspensily committing a prerendered tree ([facebook#26434](facebook/react#26434)) //<Andrew Clark>// - **[d12bdcda6](facebook/react@d12bdcda6 )**: Fix Flow types of useEffectEvent ([facebook#26468](facebook/react#26468)) //<Sebastian Silbermann>// - **[73b6435ca](facebook/react@73b6435ca )**: [Float][Fiber] Implement waitForCommitToBeReady for stylesheet resources ([facebook#26450](facebook/react#26450)) //<Josh Story>// - **[175962c10](facebook/react@175962c10 )**: Fix remaining CommonJS imports after Rollup upgrade ([facebook#26473](facebook/react#26473)) //<dan>// - **[909c6dacf](facebook/react@909c6dacf )**: Update Rollup to 3.x ([facebook#26442](facebook/react#26442)) //<Mark Erikson>// - **[9c54b29b4](facebook/react@9c54b29b4 )**: Remove ReactFabricPublicInstance and used definition from ReactNativePrivateInterface ([facebook#26437](facebook/react#26437)) //<Rubén Norte>// - **[f77099b6f](facebook/react@f77099b6f )**: Remove layout effect warning on the server ([facebook#26395](facebook/react#26395)) //<Ricky>// - **[51a7c45f8](facebook/react@51a7c45f8 )**: Bugfix: SuspenseList incorrectly forces a fallback ([facebook#26453](facebook/react#26453)) //<Andrew Clark>// - **[afb3d51dc](facebook/react@afb3d51dc )**: Fix enableClientRenderFallbackOnTextMismatch flag ([facebook#26457](facebook/react#26457)) //<Sebastian Markbåge>// - **[8e17bfd14](facebook/react@8e17bfd14 )**: Make InternalInstanceHandle type opaque in ReactNativeTypes ([facebook#26461](facebook/react#26461)) //<Rubén Norte>// - **[b93b4f074](facebook/react@b93b4f074 )**: Should not throw for children of iframe or object ([facebook#26458](facebook/react#26458)) //<Sebastian Markbåge>// - **[c0b34bc5f](facebook/react@c0b34bc5f )**: chore: update links of docs and api ([facebook#26455](facebook/react#26455)) //<Leedom>// - **[ffb6733ee](facebook/react@ffb6733ee )**: fix docs link for useSyncExternalStore ([facebook#26452](facebook/react#26452)) //<Valor(华洛)>// - **[12a1d140e](facebook/react@12a1d140e )**: Don't prerender siblings of suspended component ([facebook#26380](facebook/react#26380)) //<Andrew Clark>// Changelog: [General][Changed] - React Native sync for revisions 77ba161...ca01f35 jest_e2e[run_all_tests] bypass-github-export-checks Reviewed By: sammy-SC Differential Revision: D44669450 fbshipit-source-id: f160aad4719a00df3ceeca78d5f3fcd0aa0f8437

@josepharhar

…6524) This PR has a bunch of surrounding refactoring. See individual commits. The main change is that we no longer special case `typeof is === 'string'` as a special case according to the `enableCustomElementPropertySupport` flag. Effectively this means that you can't use custom properties/events, other than the ones React knows about on `<input is="my-input">` extensions. This is unfortunate but there's too many paths that are forked in inconsistent ways since we fork based on tag name. I think __the solution is to let all React elements set unknown properties/events in the same way as this flag__ but that's a bigger change than this flag implies. Since `is` is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work. We still support passing the `is` prop and turn that into the appropriate createElement call. @josepharhar

@josepharhar

This PR has a bunch of surrounding refactoring. See individual commits. The main change is that we no longer special case `typeof is === 'string'` as a special case according to the `enableCustomElementPropertySupport` flag. Effectively this means that you can't use custom properties/events, other than the ones React knows about on `<input is="my-input">` extensions. This is unfortunate but there's too many paths that are forked in inconsistent ways since we fork based on tag name. I think __the solution is to let all React elements set unknown properties/events in the same way as this flag__ but that's a bigger change than this flag implies. Since `is` is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work. We still support passing the `is` prop and turn that into the appropriate createElement call. @josepharhar DiffTrain build for commit 43a70a6.

sebmarkbage requested a review from sophiebits March 30, 2023 20:43

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 30, 2023

sebmarkbage force-pushed the iscustomelement branch from cfe998b to c2122f8 Compare March 30, 2023 20:56

sophiebits approved these changes Mar 30, 2023

View reviewed changes

sebmarkbage force-pushed the iscustomelement branch from c2122f8 to b34e647 Compare March 30, 2023 21:13

sebmarkbage requested a review from gaearon March 30, 2023 21:13

sebmarkbage added 5 commits March 30, 2023 17:36

Reuse setValueForAttribute for custom properties

dbaf987

Makes it clearer what the subtle difference is. Only do this when the flag is on. Otherwise, use setValueForAttribute directly

Inline createX functions into createInstance in HostConfig

63119e7

We don't need the hoistable type to use `is` since that's not supported.

Simplify custom element logic for warnings

99daab9

Since we know the namespace in this context, we don't need the special list of SVG/MathML tags that have dashes in them. Any `is` should also have a known type and use lower case for the type tag so it's irrelevant for this check.

Warn for updating the is prop in DEV

c25c735

This can only be specified once and is really technically part of the key just like the tag name. We just don't have a way to model that in React so we warn.

Rename isCustomComponent to isCustomElement

a09a9ca

That's the technical term.

sebmarkbage force-pushed the iscustomelement branch from b34e647 to 6c2a680 Compare March 30, 2023 21:36

sebmarkbage force-pushed the iscustomelement branch 2 times, most recently from e1314a2 to e1997ce Compare March 30, 2023 21:55

Fix className hydration warning for custom elements

d4e2063

sebmarkbage force-pushed the iscustomelement branch from e1997ce to d4e2063 Compare March 30, 2023 21:56

sebmarkbage merged commit 43a70a6 into facebook:main Mar 30, 2023

sammy-SC mentioned this pull request Apr 5, 2023

Use native scheduler if defined sammy-SC/react#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit the meaning of "custom element" to not include `is` #26524

Limit the meaning of "custom element" to not include `is` #26524

sebmarkbage commented Mar 30, 2023 •

edited

Loading

react-sizebot commented Mar 30, 2023 •

edited

Loading

Limit the meaning of "custom element" to not include is #26524

Limit the meaning of "custom element" to not include is #26524

Conversation

sebmarkbage commented Mar 30, 2023 • edited Loading

react-sizebot commented Mar 30, 2023 • edited Loading

Critical size changes

Significant size changes

Limit the meaning of "custom element" to not include `is` #26524

Limit the meaning of "custom element" to not include `is` #26524

sebmarkbage commented Mar 30, 2023 •

edited

Loading

react-sizebot commented Mar 30, 2023 •

edited

Loading